Conversation
|
Thanks for your contribution! |
There was a problem hiding this comment.
Pull request overview
This PR adds remote graph decomposition capability to enable running graph decomposition operations on a remote server via gRPC. The implementation includes a client module that uploads models to a remote server, an entry point module that executes the decomposition on the server side, and a test script to validate the functionality.
Changes:
- Added client-side remote graph decomposer to orchestrate remote execution
- Added server-side entry point to handle remote decomposition requests
- Added test script to validate remote graph decomposition workflow
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 17 comments.
| File | Description |
|---|---|
| graph_net/torch/remote_graph_decomposer.py | Client module that uploads models to remote server and retrieves decomposition results |
| graph_net/torch/remote_graph_decomposer_entry.py | Server-side entry point that processes remote decomposition requests and executes graph_net.torch.run_model |
| graph_net_rpc/test/test_remote_graph_decomposer.sh | Test script that validates remote graph decomposition workflow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| help="Log filename to create under $OUTPUT_WORKSPACE.", | ||
| ) | ||
| args = parser.parse_args() | ||
| main(args) |
There was a problem hiding this comment.
The main function returns an exit code (int) but it is not used. This should be sys.exit(main(args)) to properly propagate the exit code from the subprocess execution.
| main(args) | |
| sys.exit(main(args)) |
| parser.add_argument( | ||
| "--log-file", | ||
| type=str, | ||
| default="log_remote_decompose.txt", |
There was a problem hiding this comment.
The default log filename 'log_remote_decompose.txt' is inconsistent with the variable name in remote_graph_decomposer.py which defaults to 'log_remote_graph_decomposer.txt'. These should match for consistency.
| default="log_remote_decompose.txt", | |
| default="log_remote_graph_decomposer.txt", |
| log_name = candidates[0] | ||
| else: | ||
| print( | ||
| f"[remote_subgraph_decomposer] Remote log not found. expected={log_name}, candidates={candidates}", |
There was a problem hiding this comment.
The log prefix '[remote_subgraph_decomposer]' is inconsistent with the module name 'remote_graph_decomposer'. Should use '[remote_graph_decomposer]' for consistency.
| print("===== End remote log =====\n", file=sys.stderr, flush=True) | ||
| except Exception as e: | ||
| print( | ||
| f"[remote_subgraph_decomposer] Failed to decode remote log {log_name}: {e} (bytes={len(data)})", |
There was a problem hiding this comment.
The log prefix '[remote_subgraph_decomposer]' is inconsistent with the module name 'remote_graph_decomposer'. Should use '[remote_graph_decomposer]' for consistency.
| rpc_cmd = _build_remote_rpc_cmd(args) | ||
|
|
||
| print( | ||
| f"[remote_subgraph_decomposer] model_path: {model_path}", |
There was a problem hiding this comment.
The log prefix '[remote_subgraph_decomposer]' is inconsistent with the module name 'remote_graph_decomposer'. Should use '[remote_graph_decomposer]' for consistency.
| flush=True, | ||
| ) | ||
| print( | ||
| f"[remote_subgraph_decompose_entry] INPUT_WORKSPACE: {input_workspace}", |
There was a problem hiding this comment.
The log prefix '[remote_subgraph_decompose_entry]' is inconsistent with the module name 'remote_graph_decomposer_entry'. Should use '[remote_graph_decomposer_entry]' for consistency.
| flush=True, | ||
| ) | ||
| print( | ||
| f"[remote_subgraph_decompose_entry] OUTPUT_WORKSPACE: {output_workspace}", |
There was a problem hiding this comment.
The log prefix '[remote_subgraph_decompose_entry]' is inconsistent with the module name 'remote_graph_decomposer_entry'. Should use '[remote_graph_decomposer_entry]' for consistency.
| flush=True, | ||
| ) | ||
| print( | ||
| f"[remote_subgraph_decompose_entry] log_path: {log_path}", |
There was a problem hiding this comment.
The log prefix '[remote_subgraph_decompose_entry]' is inconsistent with the module name 'remote_graph_decomposer_entry'. Should use '[remote_graph_decomposer_entry]' for consistency.
| flush=True, | ||
| ) | ||
| print( | ||
| f"[remote_subgraph_decompose_entry] cmd: {' '.join(cmd)}", |
There was a problem hiding this comment.
The log prefix '[remote_subgraph_decompose_entry]' is inconsistent with the module name 'remote_graph_decomposer_entry'. Should use '[remote_graph_decomposer_entry]' for consistency.
|
|
||
| if proc.returncode != 0: | ||
| print( | ||
| f"[remote_subgraph_decompose_entry][ERROR] run_model failed with returncode={proc.returncode}. " |
There was a problem hiding this comment.
The log prefix '[remote_subgraph_decompose_entry]' is inconsistent with the module name 'remote_graph_decomposer_entry'. Should use '[remote_graph_decomposer_entry]' for consistency.
1 participant
PR Category
Feature Enhancement
Description
Add Remote Graph Decomposer